add survey to nav bar #2184
add survey to nav bar #2184
Conversation
Summary of ChangesHello @monicadsong, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on integrating a '2025 Survey' into the website's user interface. The changes aim to significantly increase the survey's visibility and encourage participation by strategically placing links within the main navigation menus and converting a key call-to-action button on the landing page to direct users to the survey. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds a link to a 2025 survey in the navigation bar across several pages. While the feature is straightforward, the implementation introduces some maintainability concerns. Specifically, the survey URL is hardcoded in multiple locations, and inline CSS is used for styling. My review provides suggestions to centralize the URL in a variable and use CSS classes for styling, which will make future updates significantly easier and aligns with best practices.
| <a class="nav-link" href="/2.18/api/" role="tab" aria-selected="">Keras 2 API documentation</a> | ||
| <a class="nav-link" href="/keras_tuner/" role="tab" aria-selected="">KerasTuner: Hyperparam Tuning</a> | ||
| <a class="nav-link" href="/keras_hub/" role="tab" aria-selected="">KerasHub: Pretrained Models</a> | ||
| <a class="nav-link" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" role="tab" aria-selected="" style="color:blue;">2025 Survey</a> |
There was a problem hiding this comment.
This line has two areas for improvement:
-
Hardcoded URL: The survey URL is hardcoded. Since this URL is used in multiple files, it's best to define it as a Jinja variable (e.g.,
survey_url) and reuse it. This will make future updates much easier. -
Inline CSS: Using
style="color:blue;"is not ideal for maintainability. It's better practice to move this styling to a CSS file. You could create a new class, for example.nav__link--highlight, in your CSS and apply it to the link.
Here's a suggested change that addresses the inline style. Please also address the hardcoded URL by defining and using a variable.
<a class="nav-link nav__link--highlight" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" role="tab" aria-selected="">2025 Survey</a>
| <li class="nav__item"><a class="nav__link" href="/keras_tuner/">KERAS TUNER</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="/keras_rs/">KERAS RS</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="/keras_hub/">KERAS HUB</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" style="color:blue;">2025 SURVEY</a></li> |
There was a problem hiding this comment.
Similar to the previous comment, this line has a hardcoded URL and an inline style.
-
Hardcoded URL: Please replace the hardcoded URL with a variable for better maintainability.
-
Inline CSS: The
style="color:blue;"should be replaced with a CSS class. You can reuse the class suggested previously (e.g.,nav__link--highlight).
<li class="nav__item"><a class="nav__link nav__link--highlight" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC">2025 SURVEY</a></li>
| <a class="nav-link" href="/2.18/api/" role="tab" aria-selected="">Keras 2 API documentation</a> | ||
| <a class="nav-link" href="/keras_tuner/" role="tab" aria-selected="">KerasTuner: Hyperparam Tuning</a> | ||
| <a class="nav-link" href="/keras_hub/" role="tab" aria-selected="">KerasHub: Pretrained Models</a> | ||
| <a class="nav-link" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" role="tab" aria-selected="">Take Our Survey!</a> |
There was a problem hiding this comment.
The survey URL is hardcoded here. To improve maintainability, please define this URL as a Jinja variable at the top of the file (e.g., {% set survey_url = '...' %}) and reference the variable here. This will make it easier to update the URL in the future, as it's used in multiple places.
| <li class="nav__item"><a class="nav__link" href="/keras_tuner/">KERAS TUNER</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="/keras_rs/">KERAS RS</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="/keras_hub/">KERAS HUB</a></li> | ||
| <li class="nav__item"><a class="nav__link" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" style="color:blue;">2025 SURVEY</a></li> |
There was a problem hiding this comment.
This line contains a hardcoded URL and an inline style, which should be addressed.
-
Hardcoded URL: Please use a variable for the URL, as mentioned in previous comments.
-
Inline CSS: The
style="color:blue;"should be replaced by a CSS class (e.g.,nav__link--highlight) for better separation of concerns and maintainability.
| <li class="nav__item"><a class="nav__link" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC" style="color:blue;">2025 SURVEY</a></li> | |
| <li class="nav__item"><a class="nav__link nav__link--highlight" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC">2025 SURVEY</a></li> |
| <div class="hero__content--wrapper"> | ||
| <div class="hero__content"> | ||
| <a class="button__round" href="/keras_3/">KERAS 3.0 RELEASED</a> | ||
| <a class="button__round" href="https://google.qualtrics.com/jfe/form/SV_5psTvNYQKv2mAwC">TAKE OUR SURVEY!</a> |
There was a problem hiding this comment.
The survey URL is hardcoded here as well. Please replace it with a reusable Jinja variable to improve maintainability.
3 participants
This change adds a link to the survey in two places: (1) nav bar (2) announcement chip, see image attached.